Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ftr:otel trace #1886

Merged
merged 6 commits into from
May 17, 2022
Merged

Ftr:otel trace #1886

merged 6 commits into from
May 17, 2022

Conversation

pherzheyu
Copy link
Contributor

@pherzheyu pherzheyu commented May 10, 2022 •
edited
Loading

What this PR does:
Support ootel trace

Which issue(s) this PR fixes:
Fixes #

You should pay attention to items below to ensure your pr passes our ci test
We do not merge pr with ci tests failed

  • All ut passed (run 'go test ./...' in project root)
  • After go-fmt ed , run 'go fmt project' using goland.
  • Golangci-lint passed, run 'sudo golangci-lint run' in project root.
  • After import formatted, (using imports-formatter to run 'imports-formatter .' in project root, to format your import blocks, mentioned in CONTRIBUTING.md above)
  • Your new-created file needs to have apache license at the top, like other existed file does.
  • All integration test passed. You can run integration test locally (with docker env). Clone our dubbo-go-samples project and replace the go.mod to your dubbo-go, and run 'sudo sh start_integration_test.sh' at root of samples project root. (M1 Slice is not Support)

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2022 •
edited
Loading

Codecov Report

Merging #1886 (1d4bc71) into 3.0 (b27ec53) will increase coverage by 0.22%.
The diff coverage is 80.00%.

❗ Current head 1d4bc71 differs from pull request most recent head 794059a. Consider uploading reports for the commit 794059a to get more accurate results

@@            Coverage Diff             @@
##              3.0    #1886      +/-   ##
==========================================
+ Coverage   44.82%   45.05%   +0.22%     
==========================================
  Files         283      287       +4     
  Lines       16933    17047     +114     
==========================================
+ Hits         7591     7681      +90     
- Misses       8533     8551      +18     
- Partials      809      815       +6
Impacted Files Coverage Δ
filter/otel/trace/filter.go 71.01% <71.01%> (ø)
filter/otel/trace/attachment.go 100.00% <100.00%> (ø)
filter/otel/trace/version.go 100.00% <100.00%> (ø)
common/proxy/proxy_factory/pass_through.go 28.84% <0.00%> (-3.07%) ⬇️
common/proxy/proxy_factory/default.go 20.28% <0.00%> (-1.25%) ⬇️
remoting/getty/listener.go 26.42% <0.00%> (-0.50%) ⬇️
remoting/xds/client.go 52.94% <0.00%> (ø)
common/proxy/proxy_factory/utils.go 0.00% <0.00%> (ø)
cluster/loadbalance/leastactive/loadbalance.go 91.66% <0.00%> (+0.36%) ⬆️
metadata/report/delegate/delegate_report.go 35.09% <0.00%> (+8.60%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b27ec53...794059a. Read the comment docs.

Lvnszn
Lvnszn reviewed May 11, 2022
View reviewed changes
filter/otel/trace/attachment.go Outdated
)

type metadataSupplier struct {
metadata *map[string]interface{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个为啥用指针，map 应该本来就是个指针吧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map是个引用类型，和指针还是有区别的，当触发map扩容时，引用地址可能会变

@AlexStocks AlexStocks merged commit 90336d9 into apache:3.0 May 17, 2022
@Lvnszn Lvnszn mentioned this pull request Jul 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LaurenceLiZhixin LaurenceLiZhixin LaurenceLiZhixin approved these changes

@Lvnszn Lvnszn Lvnszn approved these changes

@AlexStocks AlexStocks AlexStocks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pherzheyu @codecov-commenter @AlexStocks @Lvnszn @LaurenceLiZhixin